Handle lense_dir based on $::rubysitedir. #67
Conversation
|
@bzed could you please fix unit tests? Tests are passing on master so there must be something weird in your PR that make them fail. |
|
@mcanevet I gave it a short try, but I fail to understand why it fails. So please take it and fix it or leave it broken. |
| @@ -14,7 +14,11 @@ | |||
| end | |||
| end | |||
|
|
|||
| lens_dir = Puppet.version < '4.0.0' ? '/usr/share/augeas/lenses' : '/opt/puppetlabs/puppet/share/augeas/lenses' | |||
| if Puppet.version >= '4.0.0' and facts[:rubysitedir] =~ /\/opt\/puppetlabs\/puppet/ | |||
There was a problem hiding this comment.
This test is misplaced, the facts hash is not available here
| @@ -3,7 +3,11 @@ | |||
| describe 'augeas::lens' do | |||
| let (:title) { 'foo' } | |||
|
|
|||
| lens_dir = Puppet.version < '4.0.0' ? '/usr/share/augeas/lenses' : '/opt/puppetlabs/puppet/share/augeas/lenses' | |||
| if Puppet.version >= '4.0.0' and facts[:rubysitedir] =~ /\/opt\/puppetlabs\/puppet/ | |||
|
@bzed the unit tests failure definitely comes from your code. See my comments. |
|
@mcanevet see my last comment. take it, fix it, or leave it broken. I don't have the time to bother with that mess call spec tests. |
|
@bzed I think your code would work. |
|
@mcanevet well thanks, I can happily live with the broken spec tests - and with a fork. but I guess other users might be happy if they could just keep using your module. |
|
@bzed having tests that pass is a common requirement for all serious open-source projects. In the current case, it really shouldn't be too hard to fix, especially as everything is already implemented and the tests pass on the master branch. We understand other users might want to use this patch (although we don't) but we are not willing to maintain code that is untested, and even less code that breaks code that we need (even if it fixes another issue). If you need support on how to fix the tests, I'm sure you can find it on many IRC/slack Puppet channels, and we might even help you fix them if you ask, but we won't merge a PR that doesn't pass existing tests. |
Fixes: #65